No more centroiding in Simple Aperture Photometry plugin #1841
Conversation
pllim
requested review from
duytnguyendtn,
rosteen,
javerbukh,
ojustino,
kecnry and
bmorris3
as code owners
Codecov ReportBase: 88.29% // Head: 88.29% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1841 +/- ##
=======================================
Coverage 88.29% 88.29%
=======================================
Files 95 95
Lines 10396 10400 +4
=======================================
+ Hits 9179 9183 +4
Misses 1217 1217
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| x_mean = x_data[np.where(y_data == y_max)].mean() | ||
| std = 0.5 * (phot_table['semimajor_sigma'][0] + | ||
| phot_table['semiminor_sigma'][0]) | ||
| if isinstance(std, u.Quantity): | ||
| std = std.value | ||
| gs = Gaussian1D(amplitude=y_max, mean=0, stddev=std, | ||
| fixed={'mean': True, 'amplitude': True}, | ||
| gs = Gaussian1D(amplitude=y_max, mean=x_mean, stddev=std, | ||
| fixed={'amplitude': True}, |
There was a problem hiding this comment.
seems to work reasonably well for an intentionally non-centered aperture!
There was a problem hiding this comment.
It still isn't fool-proof (it will fail if y_max returns x_data values at different peaks, for instance) but hopefully work good enough for most cases that this plugin is useful for... 🤞
Simple Aperture Photometry plugin. Removed outdated known issue. Updated docs and tests.
Co-authored-by: Ricky O'Steen <[email protected]>
|
@rosteen , looks good to you know after the typo fix or is there something else? Thanks! |
Got distracted in the middle of testing, sorry. Approved! |
|
Thanks for the reviews! |
Description
This pull request is to remove partial centroiding in Simple Aperture Photometry plugin. We reintroduce fitting of
meanin Gaussian (the code was in one of the commits in #1409 but later removed before merged but still in Git history). Radial profile was added in #1030 and this plugin in #938 (there were other PRs too in between).Also removes an outdated known issue that was already fixed in #1524 .
Fixed a minor bug where Gaussian Fit section shows on GUI even when not applicable. This happens when user enables Gaussian fitting for radial profile, and then redid the calculation with Curve of Growth immediately after without disabling Gaussian fitting option.
Fixes #1442
Change log entry
CHANGES.rst? If you want to avoid merge conflicts,list the proposed change log here for review and add to
CHANGES.rstbefore merge. If no, maintainershould add a
no-changelog-entry-neededlabel.Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
triviallabel.